fix(memory): Fix memory leak for audio events when pausing the game by Caball009 · Pull Request #2731 · TheSuperHackers/GeneralsGameCode

Caball009 · 2026-05-19T14:33:00Z

Related PR fix(memory): Fix various memory leaks (2) #2710

Pausing the game leaks audio events that were in the audio request container at the time. This PR fixes that.

This code is called to get rid of some of the audio requests when pausing the game:

GeneralsGameCode/Core/GameEngineDevice/Source/MilesAudioDevice/MilesAudioManager.cpp

Lines 587 to 601 in 2219f63

    
           //Get rid of PLAY audio requests when pausing audio. 
        
           std::list<AudioRequest*>::iterator ait; 
        
           for (ait = m_audioRequests.begin(); ait != m_audioRequests.end(); /* empty */) 
        
           { 
        
           	AudioRequest *req = (*ait); 
        
           	if( req && req->m_request == AR_Play ) 
        
           	{ 
        
           		deleteInstance(req); 
        
           		ait = m_audioRequests.erase(ait); 
        
           	} 
        
           	else 
        
           	{ 
        
           		ait++; 
        
           	} 
        
           }

AudioRequest::m_pendingEvent can be an owning raw pointer, though, which the destructor of AudioRequest should delete when needed. It currently doesn't, which is why the leak happens.

GeneralsGameCode/Core/GameEngine/Include/Common/AudioRequest.h

Line 51 in 2219f63

AudioEventRTS *m_pendingEvent;

There's one exception where the ownership of the audio event is taken away from the audio request:

GeneralsGameCode/Core/GameEngineDevice/Source/MilesAudioDevice/MilesAudioManager.cpp

Line 2238 in 2219f63

processRequest(req);

greptile-apps · 2026-05-19T14:38:13Z

Greptile Summary

This PR fixes a memory leak where AudioEventRTS objects held in AudioRequest::m_pendingEvent were not deleted when audio requests were discarded during game pause, because AudioRequest had no destructor body. The fix adds a destructor that conditionally deletes m_pendingEvent and a releasePendingEvent() helper that transfers ownership to PlayingAudio, preventing double-frees.

AudioRequest destructor now deletes m_pendingEvent when m_usePendingEvent is true, covering the pause path (and the checkForSample skip path) where requests are destroyed without reaching playAudioEvent.
releasePendingEvent() clears m_usePendingEvent and nulls out m_pendingEvent before returning the pointer, so the destructor cannot double-free an event already handed off to a PlayingAudio entry.
playAudioEvent is refactored to take the full AudioRequest* and calls releasePendingEvent() at each of the three ownership-transfer sites (stream, 3D sample, 2D sample), correctly using audio->m_audioEventRTS for subsequent operations instead of the original local event pointer.

Confidence Score: 5/5

Safe to merge — the destructor fix and ownership-transfer pattern are implemented correctly across all code paths.

The destructor correctly guards with m_usePendingEvent, releasePendingEvent() atomically clears the flag before returning the pointer preventing any double-free, and all three releasePendingEvent() call sites in playAudioEvent are in mutually exclusive switch branches. The early-return path (!info) is also covered because it exits before allocatePlayingAudio() and the request destructor will clean up. No existing behavior is changed beyond the leak fix.

No files require special attention.

Important Files Changed

Filename	Overview
Core/GameEngine/Include/Common/AudioRequest.h	Adds `releasePendingEvent()` declaration to `AudioRequest`, enabling explicit ownership transfer of `m_pendingEvent`.
Core/GameEngine/Source/Common/Audio/AudioRequest.cpp	Implements the destructor fix (deletes `m_pendingEvent` when `m_usePendingEvent` is true) and adds `releasePendingEvent()` which clears the ownership flag and returns the raw pointer.
Core/GameEngineDevice/Include/MilesAudioDevice/MilesAudioManager.h	Updates `playAudioEvent` signature from `AudioEventRTS` to `AudioRequest` to allow ownership transfer inside the function.
Core/GameEngineDevice/Source/MilesAudioDevice/MilesAudioManager.cpp	Refactors `playAudioEvent` to accept the full `AudioRequest*` and call `releasePendingEvent()` at each point where ownership is transferred to `PlayingAudio::m_audioEventRTS`, covering all three sound-type branches (stream, 3D, 2D).

Sequence Diagram

sequenceDiagram
    participant PA as pauseAudio()
    participant AR as AudioRequest
    participant MAM as MilesAudioManager
    participant PlayingAudio

    note over PA,AR: Pause path (leak fixed)
    PA->>AR: deleteInstance(req) [AR_Play requests]
    AR->>AR: ~AudioRequest(): if(m_usePendingEvent) delete m_pendingEvent

    note over MAM,PlayingAudio: Normal play path (ownership transfer)
    MAM->>MAM: "processRequest(req) -> playAudioEvent(req)"
    MAM->>AR: "req->releasePendingEvent()"
    AR-->>MAM: "AudioEventRTS* (m_usePendingEvent=false, m_pendingEvent=nullptr)"
    MAM->>PlayingAudio: "audio->m_audioEventRTS = releasedEvent"
    MAM->>AR: "deleteInstance(req) [m_usePendingEvent=false, no double-free]"
    PlayingAudio->>PlayingAudio: "releasePlayingAudio() -> releaseAudioEventRTS() -> delete event"

_{Reviews (4): Last reviewed commit: "Implemented 'releasePendingEvent' member..." | Re-trigger Greptile}

xezon

Looking good.

Caball009 · 2026-05-19T22:43:51Z

Rebased to include the fix for the CI Replay checker.

xezon · 2026-05-20T13:29:47Z

On second look, it is not great that m_usePendingEvent is still true now, because other functions assume m_pendingEvent is non-null when m_usePendingEvent is true.

Can do

m_usePendingEvent = req->m_pendingEvent != nullptr;

Or maybe better, we pass AudioRequest *& to playAudioEvent and add new function:

AudioEventRTS *AudioRequest::releasePendingEvent() { if (m_usePendingEvent) { m_usePendingEvent = false; AudioEventRTS *event = m_pendingEvent; m_pendingEvent = nullptr; return event; } return nullptr; }

Then do:

audio->m_audioEventRTS = req->releasePendingEvent();

This assignment then takes care of everything.

Ok, updated.

playAudioEvent( AudioRequest* req ) should be fine, though, right?

Yes that would be sufficient then

xezon · 2026-05-20T18:12:36Z

+
+AudioEventRTS* AudioRequest::releasePendingEvent()
+{
+	DEBUG_ASSERTCRASH(m_usePendingEvent && m_pendingEvent, ("audio request was expected to contain valid audio event"));


This assert is a bit late because it would have already crashed before in MilesAudioManager::playAudioEvent if m_pendingEvent was 0.

I suggest move this assert to the begin of MilesAudioManager::playAudioEvent

xezon · 2026-05-20T18:17:55Z


 			// Put this on here, so that the audio event RTS will be cleaned up regardless.
-			audio->m_audioEventRTS = event;
+			audio->m_audioEventRTS = req->releasePendingEvent();


You could also write audio->m_audioEventRTS = event = req->releasePendingEvent(); and just keep using event everywhere.

Caball009 added Minor Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour Memory Is memory related Fix Is fixing something, but is not user facing labels May 19, 2026

Caball009 mentioned this pull request May 19, 2026

fix(memory): Fix various memory leaks (2) #2710

Open

3 tasks

Caball009 commented May 19, 2026

View reviewed changes

Comment thread Core/GameEngineDevice/Source/MilesAudioDevice/MilesAudioManager.cpp Outdated

xezon reviewed May 19, 2026

View reviewed changes

Comment thread Core/GameEngineDevice/Source/MilesAudioDevice/MilesAudioManager.cpp Outdated

Caball009 force-pushed the fix_memory_leak_audio_requests branch from 057637e to 977bd8b Compare May 19, 2026 21:15

xezon approved these changes May 19, 2026

View reviewed changes

Caball009 force-pushed the fix_memory_leak_audio_requests branch from 977bd8b to afb337c Compare May 19, 2026 22:15

fix(memory): Fix memory leak for audio events when pausing the game.

afb337c

xezon reviewed May 20, 2026

View reviewed changes

Caball009 force-pushed the fix_memory_leak_audio_requests branch from 9e09418 to 08f2330 Compare May 20, 2026 17:01

Implemented 'releasePendingEvent' member function.

08f2330

xezon reviewed May 20, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(memory): Fix memory leak for audio events when pausing the game#2731

fix(memory): Fix memory leak for audio events when pausing the game#2731
Caball009 wants to merge 2 commits into
TheSuperHackers:mainfrom
Caball009:fix_memory_leak_audio_requests

Caball009 commented May 19, 2026

Uh oh!

greptile-apps Bot commented May 19, 2026 •

edited

Loading

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

Uh oh!

Uh oh!

xezon left a comment

Uh oh!

Caball009 commented May 19, 2026

Uh oh!

xezon May 20, 2026

Uh oh!

Caball009 May 20, 2026

Uh oh!

Caball009 May 20, 2026

Uh oh!

xezon May 20, 2026

Uh oh!

xezon May 20, 2026

Uh oh!

xezon May 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	//Get rid of PLAY audio requests when pausing audio.
	std::list<AudioRequest*>::iterator ait;
	for (ait = m_audioRequests.begin(); ait != m_audioRequests.end(); /* empty */)
	{
	AudioRequest req = (ait);
	if( req && req->m_request == AR_Play )
	{
	deleteInstance(req);
	ait = m_audioRequests.erase(ait);
	}
	else
	{
	ait++;
	}
	}

Conversation

Caball009 commented May 19, 2026

Uh oh!

greptile-apps Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

Uh oh!

Uh oh!

xezon left a comment

Choose a reason for hiding this comment

Uh oh!

Caball009 commented May 19, 2026

Uh oh!

xezon May 20, 2026

Choose a reason for hiding this comment

Uh oh!

Caball009 May 20, 2026

Choose a reason for hiding this comment

Uh oh!

Caball009 May 20, 2026

Choose a reason for hiding this comment

Uh oh!

xezon May 20, 2026

Choose a reason for hiding this comment

Uh oh!

xezon May 20, 2026

Choose a reason for hiding this comment

Uh oh!

xezon May 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

greptile-apps Bot commented May 19, 2026 •

edited

Loading